rpcclient: add GetTxOutProof{,Async} methods, speed up string parsing#2489
rpcclient: add GetTxOutProof{,Async} methods, speed up string parsing#2489guggero wants to merge 2 commits intobtcsuite:masterfrom
Conversation
Roasbeef
left a comment
There was a problem hiding this comment.
Changes look good. Two main comments:
- Extract to helper function
- Tests for the expected output
|
Thanks for the review! I guess I was a bit lazy with this PR... Addressed your comments. |
|
Looks good. dd9b5e7 Also tested with a simple benchmark: It's way faster: |
|
Nice work @guggero The cast from []byte to string does a single allocation, using bytes.TrimPrefix The previous optimization gave us 49x speedup My suggestion enables 1770330x speedup The bench: func BenchmarkParseJSONString(b *testing.B) {
// Simulate a ~1 MiB serialized block hex-encoded and JSON-quoted,
// matching what bitcoind's getblock verbosity=0 returns.
raw := make([]byte, 1024*1024)
rand.New(rand.NewSource(1)).Read(raw)
payload := []byte(`"` + hex.EncodeToString(raw) + `"`)
b.Run("unmarshal", func(b *testing.B) {
b.ReportAllocs()
for i := 0; i < b.N; i++ {
var s string
if err := json.Unmarshal(payload, &s); err != nil {
b.Fatal(err)
}
}
})
b.Run("parseJSONString", func(b *testing.B) {
b.ReportAllocs()
for i := 0; i < b.N; i++ {
_ = parseJSONString(payload)
}
})
b.Run("parseJSONString2", func(b *testing.B) {
b.ReportAllocs()
for i := 0; i < b.N; i++ {
_ = parseJSONString2(payload)
}
})
}Full diff: diff --git a/rpcclient/chain.go b/rpcclient/chain.go
index bb85284a..c0d60cfa 100644
--- a/rpcclient/chain.go
+++ b/rpcclient/chain.go
@@ -6,9 +6,9 @@
package rpcclient
import (
+ "bytes"
"encoding/hex"
"encoding/json"
- "strings"
"github.com/btcsuite/btcd/btcjson"
"github.com/btcsuite/btcd/chaincfg/chainhash"
@@ -27,7 +27,7 @@ func (r FutureGetBestBlockHashResult) Receive() (*chainhash.Hash, error) {
return nil, err
}
- return chainhash.NewHashFromStr(parseJSONString(res))
+ return chainhash.NewHash(parseJSONString(res))
}
// GetBestBlockHashAsync returns an instance of a type that can be used to get
@@ -107,7 +107,7 @@ func (r FutureGetBlockResult) Receive() (*wire.MsgBlock, error) {
// Deserialize the block and return it.
var msgBlock wire.MsgBlock
- err = msgBlock.Deserialize(hex.NewDecoder(strings.NewReader(
+ err = msgBlock.Deserialize(hex.NewDecoder(bytes.NewReader(
parseJSONString(res),
)))
if err != nil {
@@ -541,7 +541,7 @@ func (r FutureGetBlockHashResult) Receive() (*chainhash.Hash, error) {
return nil, err
}
- return chainhash.NewHashFromStr(parseJSONString(res))
+ return chainhash.NewHash(parseJSONString(res))
}
// GetBlockHashAsync returns an instance of a type that can be used to get the
@@ -574,7 +574,7 @@ func (r FutureGetBlockHeaderResult) Receive() (*wire.BlockHeader, error) {
// Deserialize the blockheader and return it.
var bh wire.BlockHeader
- err = bh.Deserialize(hex.NewDecoder(strings.NewReader(
+ err = bh.Deserialize(hex.NewDecoder(bytes.NewReader(
parseJSONString(res),
)))
if err != nil {
@@ -1083,7 +1083,7 @@ func (r FutureGetTxOutProofResult) Receive() (*wire.MsgMerkleBlock, error) {
var merkleBlock wire.MsgMerkleBlock
err = merkleBlock.BtcDecode(
- hex.NewDecoder(strings.NewReader(parseJSONString(res))),
+ hex.NewDecoder(bytes.NewReader(parseJSONString(res))),
wire.ProtocolVersion, wire.WitnessEncoding,
)
if err != nil {
@@ -1212,7 +1212,7 @@ func (r FutureGetCFilterResult) Receive() (*wire.MsgCFilter, error) {
}
// Decode the serialized cf hex to raw bytes.
- serializedFilter, err := hex.DecodeString(parseJSONString(res))
+ serializedFilter, err := hex.AppendDecode(nil, parseJSONString(res))
if err != nil {
return nil, err
}
@@ -1260,7 +1260,7 @@ func (r FutureGetCFilterHeaderResult) Receive() (*wire.MsgCFHeaders, error) {
}
// Assign the decoded header into a hash
- headerHash, err := chainhash.NewHashFromStr(parseJSONString(res))
+ headerHash, err := chainhash.NewHash(parseJSONString(res))
if err != nil {
return nil, err
}
@@ -1454,11 +1454,12 @@ func (c *Client) ReconsiderBlock(blockHash *chainhash.Hash) error {
// parseJSONString parses a JSON byte slice as a JSON string by removing leading
// and trailing double quotes.
-func parseJSONString(jsonWithQuotes []byte) string {
+func parseJSONString(jsonWithQuotes []byte) []byte {
// The result is just a single hex string. So we don't need to unmarshal
// it into a string, replacing the quotes achieves the same result, just
// much faster and with fewer allocations.
- return strings.TrimPrefix(
- strings.TrimSuffix(string(jsonWithQuotes), "\""), "\"",
+ return bytes.TrimPrefix(
+ bytes.TrimSuffix(jsonWithQuotes, []byte("\"")), []byte("\""),
)
}
+
diff --git a/rpcclient/chain_test.go b/rpcclient/chain_test.go
index b37bb24c..4016e6fa 100644
--- a/rpcclient/chain_test.go
+++ b/rpcclient/chain_test.go
@@ -1,6 +1,7 @@
package rpcclient
import (
+ "bytes"
"encoding/json"
"errors"
"net/http"
@@ -274,7 +275,7 @@ func TestJSONStringParsing(t *testing.T) {
return resultStr, nil
}
- newMethod := func(res []byte) (string, error) {
+ newMethod := func(res []byte) ([]byte, error) {
return parseJSONString(res), nil
}
@@ -286,7 +287,10 @@ func TestJSONStringParsing(t *testing.T) {
newResult, err := newMethod([]byte(testCase))
require.NoError(t, err)
- require.Equal(t, oldResult, newResult)
+ if !bytes.Equal([]byte(oldResult), newResult) {
+ t.Errorf("\nexpected %s\ngot %s", oldResult,
+ newResult)
+ }
})
}
} |
This commit replaces the parsing of a JSON string with a simple prefix/suffix replacement of the quotes. That is much faster than allocating memory for a JSON parser. Where possible, we also directly decode the hex with a decoder to further reduce the number of allocations.
|
Thanks @guibressan, that's a good point. I've added a For the other occurrences, your code actually subtly breaks everything, because there is a semantic difference between |
|
Nice. In fact, the tests caught my mistake about this semantic difference. But now all the tests are passing. |
Adds a new method to fetch transaction output inclusion proofs.
While observing this code and specifically the
GetBlockmethod on mainnet, I observed a lot of JSON-related memory allocations for basically turning"<hex_string>"into<hex_string>. So I changed those JSON parse calls with simple prefix/suffix string replacements, which should hopefully be much faster and require fewer allocations.